-
Notifications
You must be signed in to change notification settings - Fork 27
Allow user to toggle Connection header #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
troylar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the error we are getting from AWS:
The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.\n\nThe Canonical String for this request should have been\n'GET\n/_cluster/health\n\naccept:application/json\nconnection:close\ncontent-type:application/json\nhost:search-poc-test-vp7ggjs3lreczj7w7b3kzs5qqy.us-east-2.es.amazonaws.com\nx-amz-date:20170810T211424Z\n\naccept;connection;content-type;host;x-amz-date\ne3b0c44298fc1c149afbf4c8996fb92427ae
| protected override System.Net.HttpWebRequest CreateHttpWebRequest(RequestData requestData) | ||
| { | ||
| var request = base.CreateHttpWebRequest(requestData); | ||
| request.Headers.Remove("Connection"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicate code, but the request object is different for both methods.
|
Any chance you're going through a proxy that might be messing with the HTTP? We are successfully connecting directly to an AWS Elasticsearch service cluster using https:// and have never encountered this. Also using Keep Alive is important for performance reasons. |
|
Interesting. Two of us are getting the same response in two different
states on two completely different networks on two completely different
clusters.
…On Aug 10, 2017 5:21 PM, "Brandon" ***@***.***> wrote:
Any chance you're going through a proxy that might be messing with the
HTTP? We are successfully connecting directly to an AWS Elasticsearch
service cluster using https:// and have never encountered this. Also
using Keep Alive is important for performance reasons.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB151FW0FSKgC6UbsPv7ubG5mcjyRRiBks5sW3RYgaJpZM4Oz_Hg>
.
|
|
You can try omitting the connection header from the signature. I don't think it needs to be part of the signature. See f3f3a52 If that fixes the issue let me know and we can include that change. I would hate to turn off keep-alive by default. |
|
100% agree with you and we can enable it by default. I don't like this
solution long term because it doesn't really address the "why" but it does
work for now.
|
|
So removing the Connection header worked as well. |
|
Cool. I'll merge that commit and publish tomorrow morning. Going to close this PR. thanks for reporting the issue. |
|
@bcuff FYI -- this where the keep-alive is being set in the Elasticsearch-net code . . .It only happens for the DOTNETCORE directive. (Line 225) |
|
And where the addition was made two months ago. Just wanted to give a full context. Thanks for making this change. |
|
Ah, that makes sense. New release is up https://www.nuget.org/packages/Elasticsearch.Net.Aws/2.3.5 |
Our requests were failing because the ElasticSearch library creates a Connection:Keep-Alive header by default. Amazon expects the request to be signed with the header Connection:close. Overriding this header allowed us to successfully query the AWS ES cluster.
This pull request simply allows you to switch between "Keep-Alive" and "close."